Skip to content

[mypyc] feat: __mypyc_empty_tuple__ constant#19654

Merged
JukkaL merged 12 commits into
python:masterfrom
BobTheBuidler:empty-tuple-constant
Aug 27, 2025
Merged

[mypyc] feat: __mypyc_empty_tuple__ constant#19654
JukkaL merged 12 commits into
python:masterfrom
BobTheBuidler:empty-tuple-constant

Conversation

@BobTheBuidler

@BobTheBuidler BobTheBuidler commented Aug 14, 2025

Copy link
Copy Markdown
Contributor

I realized that any time a user has a kwarg-only call expression like fn(abc=123, ...) in their compiled code, and func is not a native function, a new empty tuple is created every time

This is not really necessary, we can just hold the same empty tuple in memory as a constant and pass it around. It's immutable, and that's already what we're already doing, since tuple() is tuple() but our current method involves more steps.

This should slightly improve the speed of kwarg-only python func calling.

@BobTheBuidler
BobTheBuidler marked this pull request as draft August 14, 2025 06:24
@JukkaL

JukkaL commented Aug 14, 2025

Copy link
Copy Markdown
Collaborator

In 3.13 and later, we could use Py_GetConstant(Py_CONSTANT_EMPTY_TUPLE);. However, having a static variable with the value would be faster, as we'd save at least a function call, and it will work on older Python versions as well, so it's reasonable to cache it in mypyc.

@BobTheBuidler
BobTheBuidler force-pushed the empty-tuple-constant branch 2 times, most recently from 02cd112 to fecc89c Compare August 25, 2025 13:55
Comment thread mypyc/lib-rt/CPy.h Outdated
@BobTheBuidler
BobTheBuidler marked this pull request as ready for review August 25, 2025 14:09
Comment thread mypyc/lib-rt/CPy.h Outdated
Comment thread mypyc/lib-rt/init.c Outdated
PyErr_SetString(PyExc_RuntimeError, "Failed to initialize __mypyc_empty_tuple__");
return;
}
Py_INCREF(__mypyc_empty_tuple__);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to do an incref, since PyTuple_New returns a new reference that gets implicitly transferred to __mypyc_empty_tuple__.

Comment thread mypyc/lib-rt/init.c Outdated
Comment thread mypyc/lib-rt/CPy.h Outdated
@BobTheBuidler

Copy link
Copy Markdown
Contributor Author

Thanks for your feedbacks, I've updated the PR accordingly.

@BobTheBuidler
BobTheBuidler requested a review from JukkaL August 26, 2025 19:31

@JukkaL JukkaL left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found one more thing.

size: Value = Integer(len(items), c_pyssize_t_rprimitive)
return self.call_c(new_tuple_op, [size] + items, line)
else:
return self.call_c(load_empty_tuple_constant_op, [], line)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also use the new primitive in new_tuple_with_length below if length == 0.

@BobTheBuidler BobTheBuidler Aug 27, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how to implement this logic at compile time because in this location, length is a Value not a known integer

we could do a boolean op to check, but then we're just recreating what already exists in PyTuple_New and making our IR longer to do it

@JukkaL JukkaL Aug 27, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct, please disregard my above comment.

The new function could be used in mypyc/codegen/emit.py on this line when generating C for a box operation: self.emit_line(f"{declaration}{dest} = PyTuple_New({len(typ.types)});"). If length is zero, we could use the new primitive?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks correct. We can also skip the subsequent NULL check. I've implemented this and the tests are running, I may need to update the IR test definitons.

@JukkaL JukkaL left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates, looks good now!

@JukkaL
JukkaL merged commit e0ce3e3 into python:master Aug 27, 2025
13 checks passed
@BobTheBuidler
BobTheBuidler deleted the empty-tuple-constant branch August 27, 2025 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants